-
Notifications
You must be signed in to change notification settings - Fork 306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Рушкова Ольга2 #261
base: master
Are you sure you want to change the base?
Рушкова Ольга2 #261
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Почему тесты находятся не в отдельном проекте?
{ | ||
public class RectangleStorageTests | ||
{ | ||
private RectangleStorage storage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Класса не существует. Т.е фактически сломанный файл с тестами. Проект даже не сбилдится.
{ | ||
internal class CloudCompressor | ||
{ | ||
private readonly BruteForceNearestFinder nearestFinder = new (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BruteForceNearestFinder - статический класс, не может быть полем.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Соответственно проект не соберется даже
var testObj = TestContext.CurrentContext.Test.Parent?.Fixture as CircularCloudLayouterTests; | ||
var info = typeof(CircularCloudLayouterTests) | ||
.GetField("storage", BindingFlags.NonPublic | BindingFlags.Instance); | ||
var st = info?.GetValue(testObj); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А тут уместна рефлексия?
public CircularCloudLayouter(Point center) : this(center, []) | ||
{ } | ||
|
||
internal CircularCloudLayouter(Point center, List<Rectangle> storage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Протечка абстракций
private readonly List<Rectangle> storage; | ||
private readonly CloudCompressor compressor; | ||
|
||
public CircleLayer CurrentLayer { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему свойство?
private void Expand() | ||
{ | ||
Radius += DeltaRadius; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше не выносить. Чтобы лишний раз по коду не бегать
Direction.Left => (possibleNearest, rectangleForFind) => rectangleForFind.Left - possibleNearest.Right, | ||
Direction.Right => (possibleNearest, rectangleForFind) => possibleNearest.Left - rectangleForFind.Right, | ||
Direction.Top => (possibleNearest, rectangleForFind) => rectangleForFind.Top - possibleNearest.Bottom, | ||
_ => (possibleNearest, rectangleForFind) => possibleNearest.Top - rectangleForFind.Bottom, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лучше все-таки явно енам тут обрабатывать, чтобы спецэффектов не получить, если я енам расширю
return nearestByDirection.Count > 0 ? nearestByDirection.MinBy(el => el.Distance).Nearest : null; | ||
} | ||
|
||
public static Func<Rectangle, Rectangle, int> GetMinDistanceCalculatorBy(Direction direction) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Чет пересуложненное возвращаемое значение. Почему мы не можем сразу давать результат по дирекшну?
public CloudCompressor(Point compressTo, List<Rectangle> cloud) | ||
{ | ||
compressionPoint = compressTo; | ||
this.cloud = cloud; | ||
} | ||
|
||
public Rectangle CompressCloudAfterInsertion(Rectangle insertionRectangle) | ||
{ | ||
var toCompressionPoint = GetDirectionsForMovingForCompress(insertionRectangle); | ||
foreach (var direction in toCompressionPoint) | ||
{ | ||
insertionRectangle.Location = CalculateRectangleLocationAfterCompress(insertionRectangle, direction); | ||
} | ||
return insertionRectangle; | ||
} | ||
|
||
private Point CalculateRectangleLocationAfterCompress(Rectangle forMoving, Direction toCenter) | ||
{ | ||
var nearest = nearestFinder.FindNearestByDirection(forMoving, toCenter, cloud); | ||
if (nearest == null) return forMoving.Location; | ||
var distanceCalculator = nearestFinder.GetMinDistanceCalculatorBy(toCenter); | ||
var distanceForMove = distanceCalculator(nearest.Value, forMoving); | ||
return MoveByDirection(forMoving.Location, distanceForMove, toCenter); | ||
} | ||
|
||
private static Point MoveByDirection(Point forMoving, int distance, Direction whereMoving) | ||
{ | ||
var factorForDistanceByX = whereMoving switch | ||
{ | ||
Direction.Left => -1, | ||
Direction.Right => 1, | ||
_ => 0 | ||
}; | ||
var factorForDistanceByY = whereMoving switch | ||
{ | ||
Direction.Top => -1, | ||
Direction.Bottom => 1, | ||
_ => 0 | ||
}; | ||
forMoving.X += distance * factorForDistanceByX; | ||
forMoving.Y += distance * factorForDistanceByY; | ||
|
||
return forMoving; | ||
} | ||
|
||
private List<Direction> GetDirectionsForMovingForCompress(Rectangle forMoving) | ||
{ | ||
var directions = new List<Direction>(); | ||
if (forMoving.Bottom < compressionPoint.Y) directions.Add(Direction.Bottom); | ||
if (forMoving.Left > compressionPoint.X) directions.Add(Direction.Left); | ||
if (forMoving.Right < compressionPoint.X) directions.Add(Direction.Right); | ||
if (forMoving.Top > compressionPoint.Y) directions.Add(Direction.Top); | ||
return directions; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется, облако не получается достаточно плотным и круглым как раз из-за не очень оптимально выбранного алгоритма сжатия.
Если посмотреть на твои скрины, там можно увидеть пустые места куда можно двигать, но мы почему-то не двигаем.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я бы предложил двигать максимально по вектору, как достаточно простое и похожее решение.
public void CreateImage(string? filePath = null, bool withSaveSteps = false) | ||
{ | ||
var rectangles = rectangleStorage.ToArray(); | ||
|
||
using var image = new Bitmap(imageSize.Width, imageSize.Height); | ||
using var graphics = Graphics.FromImage(image); | ||
graphics.Clear(backgroundColor); | ||
graphics.DrawGrid(); | ||
var pen = new Pen(rectangleColor); | ||
|
||
for (var i = 0; i < rectangles.Length; i++) | ||
{ | ||
var nextRectangle = rectangles[i]; | ||
graphics.DrawRectangle(pen, nextRectangle); | ||
if (withSaveSteps) | ||
{ | ||
SaveImage(image, filePath); | ||
} | ||
} | ||
SaveImage(image, filePath); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Логику по рисованию и сохранению лучше разделить по SRP.
Но если не просто принципами кидаться, а раскрыть, то:
- Работа с файлами имеет много специфики. При выборе расширения, например, тебе придется расширять в том числе работу класс по работе с изображением.
- Любое расширение и кастомизация рисунка влечет за собой изменение класса по сохранению.
for (var i = 0; i < rectangles.Length; i++) | ||
{ | ||
var nextRectangle = rectangles[i]; | ||
graphics.DrawRectangle(pen, nextRectangle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Прямоугольники рисуются 1к1 к размеру изображения. Из-за этого получим, что при достаточно большом количестве прямоугольников визуализация поломается
No description provided.